Implement integer array add/sub for datetimelike indexes #19959

jbrockmendel · 2018-03-02T05:55:58Z

AFAICT this is the next-to-last non-raising case for these operations. After this is just object-dtype, and for that we already have the correct implementation in addsub_offset_array, just need to make the condition less strict and add appropriate tests.

closes #xxxx
tests added / passed
passes git diff upstream/master -u -- "*.py" | flake8 --diff
whatsnew entry

codecov · 2018-03-02T08:04:59Z

Codecov Report

Merging #19959 into master will increase coverage by <.01%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           master   #19959      +/-   ##
==========================================
+ Coverage   91.83%   91.84%   +<.01%     
==========================================
  Files         153      153              
  Lines       49498    49515      +17     
==========================================
+ Hits        45458    45475      +17     
  Misses       4040     4040

Flag	Coverage Δ
#multiple	`90.23% <100%> (ø)`	⬆️
#single	`41.87% <8.69%> (-0.02%)`	⬇️

Impacted Files	Coverage Δ
pandas/core/indexes/datetimelike.py	`96.89% <100%> (+0.09%)`	⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update cc8d33e...6cb1b43. Read the comment docs.

…tarrays4

jreback · 2018-03-07T14:15:31Z

pandas/core/indexes/datetimelike.py

+            td = Timedelta(self.freq)
+            return op(self, td * other)
+
+        else:


no need for an else

can you address this comment.

This fell through the cracks; just pushed an update.

jreback · 2018-03-07T14:16:05Z

pandas/core/indexes/datetimelike.py

@@ -810,6 +851,11 @@ def __rsub__(self, other):
                # we need to wrap in DatetimeIndex and flip the operation
                from pandas import DatetimeIndex
                return DatetimeIndex(other) - self
+            elif (is_datetime64_any_dtype(self) and hasattr(other, 'dtype') and
+                  not is_datetime64_any_dtype(other)):


can you add a comment on why this is

Addressed, but GitHub UI not surfacing.

jreback · 2018-03-07T14:16:49Z

pandas/tests/indexes/datetimes/test_arithmetic.py

+
+    @pytest.mark.parametrize('freq', ['H', 'D'])
+    @pytest.mark.parametrize('box', [np.array, pd.Index])
+    def test_dti_add_intarray_tick(self, box, freq):


we must have some tests that do index arithmetic with integers, either move them here if they are not redundant or remove them.

There are tests for integers, but nothing ATM for integer-arrays.

jreback · 2018-03-07T14:17:02Z

pandas/tests/indexes/period/test_arithmetic.py

+    # ---------------------------------------------------------------
+    # __add__/__sub__ with integer arrays
+
+    @pytest.mark.parametrize('box', [np.array, pd.Index])


same comment as above

jbrockmendel · 2018-03-07T16:43:30Z

Comments above acknowledged.

Let's stick a pin in this for a little bit. I think there may be a problem with integer ops, need to double-check before moving forward on this.

…tarrays4

jbrockmendel · 2018-03-08T01:00:08Z

Let's stick a pin in this for a little bit. I think there may be a problem with integer ops, need to double-check before moving forward on this.

I've convinced myself this is OK. Or more accurately, that weird behavior (not affected by this PR, but in integer add/sub) can only come up if a user specifically passes verify_integrity=False to DatetimeIndex or TimedeltaIndex.

…tarrays4

jbrockmendel · 2018-03-18T17:18:15Z

Needs rebasing since its gotten a bit stale, but I think this is otherwise ready. Will push shortly.

…tarrays4

jbrockmendel · 2018-03-28T03:26:24Z

@jreback gentle ping

jreback · 2018-03-28T10:38:23Z

rebase as well

…tarrays4

jbrockmendel · 2018-04-06T18:47:29Z

@jreback gentle ping. I've got some time this weekend I can put towards finishing this off.

…tarrays4

jbrockmendel · 2018-04-16T22:43:51Z

@gfyoung any idea if everything/everyone is OK?

gfyoung · 2018-04-16T22:46:51Z

@jbrockmendel : I see a bunch of comments from @jreback that you haven't explicitly addressed. That might be why no one is saying anything.

jbrockmendel · 2018-04-16T22:51:00Z

@gfyoung Thanks. I think there's just the one where I haven't un-indented an else: clause as requested; might as well do that now. I think the others are addressed, though some are indirect.

gfyoung · 2018-04-16T22:54:38Z

I think the others are addressed, though some are indirect.

~~Did you address the commenting request above?~~

EDIT: I see the comment now! 😄

gfyoung · 2018-04-16T22:56:18Z

@jreback : ping. I think all comments have been addressed.

…tarrays4

jbrockmendel · 2018-04-26T22:11:20Z

@jreback gentle ping. I'd like to finish this off and get back to unifying Index/Series/DataFrame arithmetic implementations in core.ops.

jreback · 2018-04-26T23:20:26Z

will have to wait until after the rc

…tarrays4

jbrockmendel · 2018-05-18T22:42:33Z

@jreback gentle ping

jreback · 2018-05-18T23:19:29Z

doc/source/whatsnew/v0.23.0.txt

@@ -951,7 +951,7 @@ Datetimelike API Changes
  rather than ``NotImplementedError`` when index is not a :class:`DatetimeIndex` (:issue:`20725`).
 - Restricted ``DateOffset`` keyword arguments. Previously, ``DateOffset`` subclasses allowed arbitrary keyword arguments which could lead to unexpected behavior. Now, only valid arguments will be accepted. (:issue:`17176`, :issue:`18226`).
 - :func:`pandas.merge` provides a more informative error message when trying to merge on timezone-aware and timezone-naive columns (:issue:`15800`)
- For :class:`DatetimeIndex` and :class:`TimedeltaIndex` with ``freq=None``, addition or subtraction of integer-dtyped array or ``Index`` will raise ``NullFrequencyError`` instead of ``TypeError`` (:issue:`19895`)
+- For :class:`DatetimeIndex` and :class:`TimedeltaIndex` with ``freq=None``, addition or subtraction of integer-dtyped array or ``Index`` will raise ``NullFrequencyError`` instead of ``TypeError`` if the index ``freq`` attribute is ``None``, and will return an object of the same class otherwise (:issue:`19895`, :issue:`19959`)


0.24 (this is an api change right?)

Yes, just changed.

…tarrays4

jbrockmendel · 2018-05-26T18:28:17Z

@jreback if we can get this through then DatetimeIndex + IntArray will be meaningful and we can flesh out the tests in #21160.

jreback · 2018-05-29T01:43:17Z

thanks @jbrockmendel

it seems there might be some code you can remove from PeriodIndex though? (obviously in a followup)

jbrockmendel · 2018-05-29T03:47:33Z

it seems there might be some code you can remove from PeriodIndex though?

Sounds plausible; I'll have to refresh my memory. IIRC the next dtype to handle is Period/PeriodIndex subtraction, with the complication that we need to fix the current scalar subtraction behavior.

…19959)

jbrockmendel added 2 commits March 1, 2018 21:53

implement integer array add/sub for datetimelike indexes

f1147af

whatsnew, gh references in tests

39d8ee7

Merge branch 'master' of https://github.com/pandas-dev/pandas into in…

8780466

…tarrays4

jbrockmendel force-pushed the intarrays4 branch from a8c5c75 to 8780466 Compare March 2, 2018 17:18

jreback requested changes Mar 7, 2018

View reviewed changes

jreback added Datetime Datetime data dtype Timedelta Timedelta data type Period Period data type Compat pandas objects compatability with Numpy or Python functions labels Mar 7, 2018

Merge branch 'master' of https://github.com/pandas-dev/pandas into in…

f4e8e01

…tarrays4

jbrockmendel added 2 commits March 7, 2018 17:10

add comment on __rsub__

70f359d

Merge branch 'master' of https://github.com/pandas-dev/pandas into in…

79e7575

…tarrays4

Merge branch 'master' of https://github.com/pandas-dev/pandas into in…

ef6acdb

…tarrays4

jbrockmendel added 2 commits March 30, 2018 09:13

Merge branch 'master' of https://github.com/pandas-dev/pandas into in…

d477304

…tarrays4

Merge branch 'master' of https://github.com/pandas-dev/pandas into in…

df84dc6

…tarrays4

Merge branch 'master' of https://github.com/pandas-dev/pandas into in…

e4b6ec8

…tarrays4

unindent because some people have no sense of pizazz

be737e0

Merge branch 'master' of https://github.com/pandas-dev/pandas into in…

650b1ef

…tarrays4

Merge branch 'master' of https://github.com/pandas-dev/pandas into in…

671a008

…tarrays4

jbrockmendel mentioned this pull request Apr 26, 2018

ENH: implement DatetimeLikeArray #19902

Merged

Merge branch 'master' of https://github.com/pandas-dev/pandas into in…

bcf2419

…tarrays4

jreback requested changes May 18, 2018

View reviewed changes

jbrockmendel added 2 commits May 19, 2018 11:57

Merge branch 'master' of https://github.com/pandas-dev/pandas into in…

8cc5270

…tarrays4

Move/fix whatsnew note to 0.24.0

6cb1b43

jbrockmendel mentioned this pull request May 29, 2018

implement arith ops on pd.Categorical #21213

Closed

4 tasks

jreback added this to the 0.24.0 milestone May 29, 2018

jreback approved these changes May 29, 2018

View reviewed changes

jreback merged commit b64e9d5 into pandas-dev:master May 29, 2018

jbrockmendel deleted the intarrays4 branch May 29, 2018 03:47

david-liu-brattle-1 pushed a commit to david-liu-brattle-1/pandas that referenced this pull request Jun 18, 2018

Implement integer array add/sub for datetimelike indexes (pandas-dev#…

d236969

…19959)

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Implement integer array add/sub for datetimelike indexes #19959

Implement integer array add/sub for datetimelike indexes #19959

jbrockmendel commented Mar 2, 2018 •

edited

Loading

codecov bot commented Mar 2, 2018 •

edited

Loading

jreback Mar 7, 2018

jreback Mar 28, 2018

jbrockmendel Apr 16, 2018

jreback Mar 7, 2018

gfyoung Apr 16, 2018

jreback Mar 7, 2018

jbrockmendel Mar 8, 2018

jreback Mar 7, 2018

jbrockmendel commented Mar 7, 2018

jbrockmendel commented Mar 8, 2018

jbrockmendel commented Mar 18, 2018

jbrockmendel commented Mar 28, 2018

jreback commented Mar 28, 2018

jbrockmendel commented Apr 6, 2018

jbrockmendel commented Apr 16, 2018

gfyoung commented Apr 16, 2018 •

edited

Loading

jbrockmendel commented Apr 16, 2018

gfyoung commented Apr 16, 2018 •

edited

Loading

gfyoung commented Apr 16, 2018 •

edited

Loading

jbrockmendel commented Apr 26, 2018

jreback commented Apr 26, 2018

jbrockmendel commented May 18, 2018

jreback May 18, 2018

jbrockmendel May 19, 2018

jbrockmendel commented May 26, 2018

jreback commented May 29, 2018

jbrockmendel commented May 29, 2018

Implement integer array add/sub for datetimelike indexes #19959

Implement integer array add/sub for datetimelike indexes #19959

Conversation

jbrockmendel commented Mar 2, 2018 • edited Loading

codecov bot commented Mar 2, 2018 • edited Loading

Codecov Report

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jbrockmendel commented Mar 7, 2018

jbrockmendel commented Mar 8, 2018

jbrockmendel commented Mar 18, 2018

jbrockmendel commented Mar 28, 2018

jreback commented Mar 28, 2018

jbrockmendel commented Apr 6, 2018

jbrockmendel commented Apr 16, 2018

gfyoung commented Apr 16, 2018 • edited Loading

jbrockmendel commented Apr 16, 2018

gfyoung commented Apr 16, 2018 • edited Loading

gfyoung commented Apr 16, 2018 • edited Loading

jbrockmendel commented Apr 26, 2018

jreback commented Apr 26, 2018

jbrockmendel commented May 18, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jbrockmendel commented May 26, 2018

jreback commented May 29, 2018

jbrockmendel commented May 29, 2018

jbrockmendel commented Mar 2, 2018 •

edited

Loading

codecov bot commented Mar 2, 2018 •

edited

Loading

gfyoung commented Apr 16, 2018 •

edited

Loading

gfyoung commented Apr 16, 2018 •

edited

Loading

gfyoung commented Apr 16, 2018 •

edited

Loading